Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add _exptl absorpt.coefficient_mu_su #431

Merged

Conversation

rowlesmr
Copy link
Collaborator

Prompted by discussion #430 (comment)

_exptl_absorpt.coefficient_mu was defined as Number, but the description of this item is given in terms of values which are Measurands. _exptl_absorpt.coefficient_mu was changed to Derived Measurand, and the SU data item was added.

In the process, I noticed that a bunch of SU data items' _type.source was Related, which doesn't seem to link up with the description of Related, so I changed them all to match the _type.source of the data item to which the SU belongs.

…tch _cell_measurement.pressure"

This reverts commit ab7907f.
@rowlesmr rowlesmr changed the title Add _exptl absorpt.coefficient_mu_su and fix _type.source of many other data items Add _exptl absorpt.coefficient_mu_su Jun 26, 2023
@rowlesmr
Copy link
Collaborator Author

rowlesmr commented Jun 26, 2023

I checked the DDL1 dictionary and the _exptl_absorpt_coefficient_mu data item was also not allowed to have SU. Maybe there is a legitimate reason for that after all?

The EXPTL category is

The CATEGORY of data items used to specify the experimental work
prior to diffraction measurements. These include crystallization
crystal measurements and absorption-correction techniques used.

_exptl_absorpt.coefficient_mu is

Absorption coefficient mu calculated from the atomic content of
the cell, the density and the radiation wavelength.

So, the value of _exptl_absorpt.coefficient_mu should be calculated from things you know before the diffraction experiment. This ties in with it's Recorded status. It still doesn't mean that it shouldn't have an SU - the density, at least, would have an associated uncertainty. Being in EXPTL doesn't preclude having an SU; _exptl.transmission_factor_max, _exptl_crystal.density_meas, and many others have SUs.

.

Branching slightly out-of-scope:
The use of EXPTL for "experimental work prior to diffraction measurements" is then muddied by _exptl_crystal.density_diffrn, where the dREL involves _cell.volume, which is a diffraction experiment thing. Although, there is _exptl_crystal.density_meas for "density measured using standard chemical and physical methods". _exptl_crystal.density_diffrn is the only data item (at all) to have "_diffrn" appended to it, which I'm guessing identifies it as a data item in the exptl category, which is actually made up of diffraction-related information.

Maybe related, is that _cell.atomic_mass doesn't have an SU, as the atomic mass is calculated from ATOM_TYPE loops, not ATOM_SITE. Which also means you can't report an atomic mass of a unit cell from a refined structure.

@jamesrhester
Copy link
Contributor

Because the link from SU to the item it is the su of is specified via _name.linked_item_id, the _type.source is Related instead of derived. I note that this is not actually stated anywhere in ddl.dic, so perhaps we should rethink that or improve the Related description.

Regarding mu, I think it is entirely reasonable to allow it to have an su.

@rowlesmr
Copy link
Collaborator Author

Because the link from SU to the item it is the su of is specified via _name.linked_item_id, the _type.source is Related instead of derived. I note that this is not actually stated anywhere in ddl.dic, so perhaps we should rethink that or improve the Related description.

This is a wide ranging change. The general_su save frame gives everything _type.source Derived.

I am not qualified to properly parse the description of Related, but it does seem to only refer to looped lists.

# just FYI
         Related
;
         A value or tag used in the construction of looped lists of data.
         Typically identifying an item whose unique value is the reference key
         for a loop category and/or an item which has values in common with
         those of another loop category and is considered a Link between these
         lists.
;

Copy link
Collaborator

@vaitkus vaitkus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can merge this PR as is. The _type.source issue can be further discussed in other issue and PRs that are already open.

@jamesrhester jamesrhester merged commit 69a6ff8 into COMCIFS:master Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants